Add unique id base methods - #62
Conversation
There was a problem hiding this comment.
Can we move the id property to Definition model? Is there any Definition subclass that does not have the id property?
There was a problem hiding this comment.
ComponentDefinition should be keyed by name.
I don't understand, what do you mean by hooking ids? What are you trying to accomplish? |
|
Once this is landed, we need to fix the way how ModelAccessControl ids are used and modify the serialization to remove them from the JSON output (see #53) |
There isn't a hook in loopback to allow us to set the ID using a prototype method. We have |
Keep in mind that |
|
@bajtos can we land this first? If it looks good to you and I am not around please merge. |
There was a problem hiding this comment.
You have dropped the debug statement, is it intentional? The same question applies to the Array.isArray branch above.
There was a problem hiding this comment.
The last item has a trailing comma - the JSON is malformed.
|
@ritch The patch does not merge cleanly with the current 3.0, could you please rebase and resolve conflicts? When I attempted to make the rebase myself, I arrived at code that was not passing all tests. |
There was a problem hiding this comment.
This seems reduntant, as entity is not used anywhere AFAICT.
I would be also worried about creating Models at the point when the connector cache is in a possibly inconsistent state.
|
Two more comments. Other than that, the patch LGTM. Feel free to merge. |
- allow `WorkspaceEntity` to determine the unique id - `connector.create` should callback with an id - all models should not have the correct `id` property - pass the correct identifier data to `addRelatedCache` so it can set the `componentName` and the correct foreign key - clean related data for both embedded arrays and objects - remove `componentName` and `id` for all embedded config data - ensure the value being set to the cache has the correct id - add id tests (could use more for other model types)
- rename model foreign key - remove modelName property from all entities - remove componentName from entities where it was made redundant by the parent id (eg. modelProperty.modelId has the componentName) - allow for simpler id building by adding belongsTo relationships - use belongs to relations to build the id - parentIds (belongsTo foreignKeys) are now required - cleanup data (eg. dataSourceDef.id) leaking into config files
|
@bajtos rebased and all tests are passing. Had to fix a couple minor things. |
2 participants
/to @bajtos
This refactors all identifiers for workspace entities. It uses the
idName(model)and provides the right value based on the configuration of the entity (inmodels.json). By default it will setidName = componentName+nameIn
models.jsonwe can set an additional id part to create a more specific id when required.componentName+modelName+name